fix: tighten tag-value validation; auto-capture repository from GITHUB_REPOSITORY#12
Merged
Merged
Conversation
…B_REPOSITORY Bug - The Spice Cloud API rejects tag values that contain anything besides alphanumerics and `_@-`, but the action only validated value length. A user passing `tags: |\n repository: owner/repo` would hit the API with a 400 Bad Request mid-deploy. Add a local value pattern check matching the API rule so the failure surfaces locally with a clear error before any state-changing call. Auto-capture - When `GITHUB_REPOSITORY` is set (always true on GitHub-hosted runners), the action now auto-adds a `repository` tag derived from that env var, rewriting `/` to `_` so the value passes API validation. User-supplied `repository:` in the `tags` input still wins on conflict. - New `sanitizeTagValue`, `deriveDefaultTags`, and `mergeWithDefaultTags` helpers in `src/tags.ts`, all unit-tested. Other - Drop the post-update mutation of `app.tags` in `maybeUpdateAppMetadata`. Nothing downstream reads it, and the mutation made shared `App` objects in tests leak state across cases.
There was a problem hiding this comment.
Pull request overview
This PR tightens app-tag handling in the deploy action by validating tag values against the API’s allowed character set, auto-deriving a default repository tag from GITHUB_REPOSITORY, and removing a test-only state leak from app-tag mutation. It fits into the action’s deployment flow by moving tag failures earlier and enriching app metadata automatically.
Changes:
- Add local tag-value validation plus helpers to sanitize and derive default tags from GitHub environment data.
- Apply default-tag merging during app creation and app metadata updates in the deploy flow.
- Expand unit tests and update user-facing docs/changelog for the new tag behavior.
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/tags.ts |
Adds tag-value validation and helper functions for sanitizing and deriving default tags. |
src/deploy.ts |
Wires default-tag merging into app creation/update logic and removes post-update tag mutation. |
__tests__/tags.test.ts |
Adds unit coverage for stricter validation and default-tag helper behavior. |
__tests__/deploy.test.ts |
Adds deploy-flow tests for auto-captured and user-overridden repository tags. |
README.md |
Documents stricter tag rules and automatic repository tagging. |
CHANGELOG.md |
Records the new validation and default-tag behavior under Unreleased. |
Comments suppressed due to low confidence (2)
src/deploy.ts:139
- Because the synthesized default tags are spread after
app.tags, an existingapp.tags.repositorywill be overwritten even when the user did not supply anyrepository:tag. That turns the new default into a silent metadata rewrite for existing apps instead of only filling a missing value.
const merged = { ...(app.tags ?? {}), ...tags };
const update: UpdateAppBody = { tags: merged };
src/deploy.ts:114
- The new default-tag behavior on the create-app path is not covered by the deploy tests. The added end-to-end tests only assert the
updateApppath, so a regression wherecreateAppstops forwarding the synthesizedrepositorytag would currently slip through.
const tags = mergeWithDefaultTags(parseTags(inputs.tagsRaw));
core.info(`App "${inputs.appName}" not found; creating in region "${inputs.region}".`);
return api.createApp({
name: inputs.appName,
region: inputs.region,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| > | ||
| > **Tag value rule:** the Spice Cloud API allows only alphanumerics and `_@-`. The action validates this locally so you fail fast with a clear error instead of a `400 Bad Request` on the server. | ||
| > | ||
| > **Auto-captured tags:** when `GITHUB_REPOSITORY` is set (always true on GitHub-hosted runners), the action adds a `repository` tag derived from that env var, with `/` rewritten to `_` so the value passes API validation. Setting `repository:` explicitly in your `tags` overrides the auto-captured value. |
| | `region` | conditional | — | Spice Cloud region (e.g. `us-east-1`, `us-west-2`). Required for new apps. | | ||
| | `visibility` | no | `private` | `public` or `private` — only used on app creation. | | ||
| | `tags` | no | — | YAML or JSON map of tag key/value pairs. Merged into existing app tags. | | ||
| | `tags` | no | — | YAML or JSON map of tag key/value pairs. Merged into existing app tags. Tag values may contain only alphanumerics and `_@-`. The action auto-adds a `repository` tag from `GITHUB_REPOSITORY` (sanitizing `/` → `_`) unless you override it. | |
Comment on lines
+124
to
+125
| export function sanitizeTagValue(value: string): string { | ||
| return value.replace(/[^A-Za-z0-9_@-]/g, "_").slice(0, MAX_VALUE_LENGTH); |
Comment on lines
+135
to
137
| const tags = mergeWithDefaultTags(parseTags(inputs.tagsRaw)); | ||
| if (!tags) return; | ||
|
|
3 tasks
…tion-and-auto-repo
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two related fixes prompted by a real failed run:
```
PUT /v1/apps/4573 failed: 400 Bad Request — Tag validation failed:
Invalid value for key "repository": Must contain only alphanumeric
characters and the symbols _@-
```
The user's workflow set `tags: "repository: lukekim/home"` and the API rejected the `/` mid-deploy.
1. Tighten tag-value validation locally
The Spice Cloud API allows only alphanumerics and `_@-` in tag values, but the action only enforced length. Add a local value pattern check matching the API rule so the failure surfaces with a clear actionable error before any state-changing API call:
```
tags: value for "repository" must contain only letters, numbers, and "_@-" (got "lukekim/home").
```
2. Auto-capture `repository` from `GITHUB_REPOSITORY`
When `GITHUB_REPOSITORY` is set (always true on GitHub-hosted runners), the action now auto-adds a `repository` tag derived from that env var, rewriting `/` to `_` so the value passes API validation. So a workflow on `lukekim/home` will now have its app tagged with `repository: lukekim_home` for free, with no `tags:` input required. Setting `repository:` explicitly in the user's `tags` input still wins on conflict — no surprise overrides.
New helpers in `src/tags.ts`, all unit-tested:
3. Drive-by
Drop the post-update mutation of `app.tags` in `maybeUpdateAppMetadata`. Nothing downstream reads it, and the mutation made shared `App` objects in tests leak state across cases.
Test plan
Out of scope